-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update(android): android 13 camera open Fixed #839
Conversation
why apache so slowly merge this ,I need it ,ばかやろ |
String version = android.os.Build.VERSION.RELEASE; | ||
if(Integer.parseInt(version) <= 13) | ||
{ | ||
if(PermissionHelper.hasPermission(this, Manifest.permission.CAMERA)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: have you tested this line, because in my test in android 13 hasPermission is every time false, or you call a request to have permission, or remove this line(my android 13 worked without this if)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CAMERA
permission is normally not required, but there is a caveat with the ACTION_MEDIA_CAPTURE API.
Note: if you app targets M and above and declares as using the Manifest.permission.CAMERA permission which is not granted, then attempting to use this action will result in a SecurityException.
Normally the CAMERA
permission is only for gaining access to the camera hardware so it's not normally needed when intents. But if something, or someone adds the CAMERA
permission to the app's manifest, then the CAMERA
permission needs to be granted for the intent to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With that being said, the condition seems to be doing <=
android 13 check. But this caveat applies if your app targets M
(API 23) or later, which is virtually all apps today. So I think this check will fail if you have an app that declares the CAMERA
permission on the next Android version.
On 21-Aug-2023 12:59 pm, Francesco ***@***.***> wrote:
@spiderciccio commented on this pull request.
________________________________
In src/android/CameraLauncher.java<#839 (comment)>:
@@ -1312,20 +1312,39 @@ public void onScanCompleted(String path, Uri uri) {
}
public void onRequestPermissionResult(int requestCode, String[] permissions,
- int[] grantResults) {
- for (int r : grantResults) {
- if (r == PackageManager.PERMISSION_DENIED) {
- this.callbackContext.sendPluginResult(new PluginResult(PluginResult.Status.ERROR, PERMISSION_DENIED_ERROR));
- return;
+ int[] grantResults) throws JSONException {
+ String version = android.os.Build.VERSION.RELEASE;
+ if(Integer.parseInt(version) <= 13)
+ {
+ if(PermissionHelper.hasPermission(this, Manifest.permission.CAMERA)) {
question: have you tested this line, because in my test in android 13 hasPermission is every time false, or you call a request to have permission, or remove this line(my android 13 worked without this if)
—
Reply to this email directly, view it on GitHub<#839 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A5WIOIEFJRK2IHVF2OBJUQDXWME4XANCNFSM6AAAAAA2LIFRYU>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
Maybe your camera permission is issue
|
return; | ||
int[] grantResults) throws JSONException { | ||
String version = android.os.Build.VERSION.RELEASE; | ||
if(Integer.parseInt(version) <= 13) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check may fail, you cannot assume that the user-visible will be number like or an integer.
Instead it should use Build.VERSION.SDK_INT
Which refers to the API level. Android 13 is API 33, which has a constant Build.VERSION_CODES.TIRAMISU
Can you also explain the rationale for adding in a version check for Android 13 and earlier? This is effectively selecting all devies that we support, excluding any newer android versions in the future. So that seems strange to me.
This PR is superseded by Should be fixed by #844 |
Platforms affected
Not Affected any Platform
Description
Android 13 camera not open issue
closes #840
Testing
I was checked Manually
Checklist